Two minor cleanups:

1. bijection_type is typed as str on the attack but BijectionType on the converter. Line 42 should be bijection_type: BijectionType = BijectionType.LETTER so the public‑facing attack matches the converter's signature. The StrEnum still accepts the literal "letter" at runtime, but the type annotation lies as written.

2. Optional[X] instead of X | None. Lines 37–39 use Optional[AttackConverterConfig], Optional[AttackScoringConfig], Optional[PromptNormalizer]. The codebase enforces PEP 604 (X | None) via ruff UP007/UP045 — pre‑commit will catch these. While you're at it, line 6 can drop Optional from the typing import.

Blocking — mutating result.last_response.original_value corrupts the audit trail.

result.last_response.original_value = decoded
result.last_response.original_value = decoded

last_response is a reference to a Message that has already been written to CentralMemory by PromptSendingAttack._perform_async. This in‑place mutation overwrites the recorded target response so memory now shows the decoded plain‑English text as if the target had returned it directly — the actual cipher‑text response from the model is lost from the audit log.

For an attack whose entire purpose is to produce harmful content in obfuscated form, losing the real model output is a significant integrity problem: future runs can't be replayed, the cipher‑shape (which is the evidence the attack worked) is gone, and any downstream analysis sees only the post‑processed version.

The decoded value should be attached alongside the original, e.g.:

• Add a converted MessagePiece (preferred — that's what the converter pipeline normally produces, and it's what response converters in the normalizer do automatically).
• Or store the decoded text in AttackResult metadata (result.metadata["decoded_response"] = decoded) and leave original_value untouched.

Related: this is another argument for letting the converter pipeline handle decoding (via response converters on the normalizer) rather than doing it manually here — the pipeline already preserves the original and adds converted values without mutation.

Brittle assertion — "secret code" is a literal string from the intro message that's quite likely to get reworded (e.g., if you switch the mapping turn to a system prompt per the paper, the wording will almost certainly change). The test will then break for a reason unrelated to what it's actually trying to verify.

Better to assert structural properties:

• the first message has role="user" (or "system" after the fix)
• the message count matches 1 + 2 * num_teaching_shots (intro + shot pairs)
• subsequent messages alternate roles
• shots contain the encoded form of examples[i]

These would catch real regressions (e.g., the alternating‑roles fix being undone) instead of just minor prompt rewording.

Blocking — assistant teaching turns are plaintext English, not in‑cipher.

This change addresses my earlier comment about alternating roles, but the actual content of the assistant turns defeats the purpose. The paper (§2) specifies "in‑context User‑Assistant shots, with User messages in English and Assistant messages in the corresponding bijection language 'translation'". The current code does the opposite:

# line 92-95: ACK in plaintext English
"Understood! I will use this secret code in our conversation."

# line 109-119: user sends cipher and asks for confirmation,
# assistant replies in a half-cipher/half-English translation echo
f"In our code '{encoded}' means '{original}'. Understood?"   # user
f"{encoded} = {original}. Got it!"                            # assistant

The mechanism that makes the attack work is the assistant fluently producing cipher output — that's what induces the cipher‑shaped response distribution at inference time. Plain‑English ACKs plus cipher = plain translation echoes look to the model like "the user is showing me a translation key," not "I should produce text in this language."

Per the paper, the shot pattern should be:

• User (English): "the quick brown fox"
• Assistant (cipher): "ekt cvpjl mryio gyx"

And the ACK turn isn't needed at all — the paper just uses 10 translation shots, no separate acknowledgment.

Restructure recommendation: abstract BijectionConverter + 3 concrete subclasses, attack takes a converter instance.

After rereading the paper, the current single-class + BijectionType StrEnum design doesn't scale to what the paper actually requires. §2 specifies three bijection types — permuted alphabet, ℓ‑digit numbers, and tokens from the target's tokenizer — and explicitly notes their complexity parameters (fixed_size, ℓ, vocab subset) are what give the attack its scale‑adaptive property. So implementing only LETTER understates what the attack claims to do.

Stuffing per‑mode params (num_digits for digits, tokenizer for tokens) onto a single class produces dead‑param footguns (BijectionConverter(bijection_type=DIGITS, fixed_size=5) would silently mix modes). Subclasses give honest signatures:

class BijectionConverter(PromptConverter, abc.ABC):
    def __init__(self, *, mapping: dict[str, str] | None = None, seed: int | None = None) -> None:
        rng = random.Random(seed)
        self._mapping = mapping if mapping is not None else self._generate_mapping(rng)
        self._inverse_mapping = {v: k for k, v in self._mapping.items()}

    @abc.abstractmethod
    def _generate_mapping(self, rng: random.Random) -> dict[str, str]: ...

    async def convert_async(...): ...   # shared
    def decode(...): ...                # shared
    def _build_identifier(...): ...     # shared

class LetterBijectionConverter(BijectionConverter):
    def __init__(self, *, fixed_size: int = 0, mapping=None, seed=None): ...

class DigitBijectionConverter(BijectionConverter):
    def __init__(self, *, num_digits: int = 2, mapping=None, seed=None): ...

class TokenBijectionConverter(BijectionConverter):
    def __init__(self, *, tokenizer, mapping=None, seed=None): ...

The base class gets two things that are needed now, not just for future modes:

• seed — currently random.shuffle uses the global RNG with no way to reproduce a mapping. Red‑team work needs replay; a seed parameter (constructing a local random.Random(seed)) is the standard fix.
• mapping — accept an explicit dict[str, str] so callers can replay a known successful mapping or run deterministic experiments. The test file currently works around the lack of this by reading converter.mapping after random generation, which is awkward.

BijectionAttack then simplifies dramatically — it just accepts a converter instance:

class BijectionAttack(PromptSendingAttack):
    def __init__(
        self,
        *,
        objective_target: PromptTarget = REQUIRED_VALUE,
        bijection_converter: BijectionConverter = REQUIRED_VALUE,  # this could also be None and have the letter version as default
        num_teaching_shots: int = 10,
        ...
    ): ...

This drops bijection_type, fixed_size, and the type‑confused bijection_type: str = "letter" annotation. The user composes:

attack = BijectionAttack(
    objective_target=target,
    bijection_converter=DigitBijectionConverter(num_digits=2, seed=42),
)

I'd push for all three modes in this PR — landing only LETTER and adding the rest later means either a breaking API change (when the inevitable bijection_type / per‑mode params get reshuffled) or sticking with the current sub‑optimal design. The architectural cost is paid once now; the alternative is paying it twice.

Acknowledging this is a bigger ask than my earlier comments — happy to discuss if you'd prefer to land LETTER only and follow up, but I think the restructure is worth it.